Skip to content

Conversation

@eliseius
Copy link

Исправил замечания. Переделал и сократил решение Задания №5

@eliseius eliseius marked this pull request as draft June 20, 2023 20:49
@eliseius eliseius marked this pull request as ready for review June 20, 2023 20:49
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В коменте к пул-реквесту написано, что поправил пятое задание из бонусных, а в пул-реквесте вообще все задания. Непонятно что смотреть.

Я погляжу всё, но старайся пжл чтобы в пул-реквестах не было того, что смотреть не надо.

for inf_student in students:
student_name = inf_student['first_name']
if student_name in number_repetitions_names:
number_repetitions_names[student_name] = number_repetitions_names.get(student_name) + 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут не нужно использовать get тк в условии ифа ты проверил, что такой ключ точно есть

class_students = {}
for every_schoolkid in every_class:
if every_schoolkid['first_name'] in class_students:
count_name += 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В этой переменной какая-то беда: ты вроде считаешь повторение одного имени, но переменная общая на все имена: обнуляется только перед первым циклом.

Я бы советовал вообще избавиться от этой переменной.

return one_message['sent_by']


# Решение задания №3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Лишний коммент



def message_with_max_viewing(messages):
users_with_users = create_users_with_unique_users(messages)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

По названию непонятно, что в этой переменной. По названию функции тоже непонятно, что она делает.

"Создать пользователей с уникальными пользователями"?..


def get_id_last_messages_in_thread(id_with_reply_for):
id_message_for_reply = [id for id in id_with_reply_for
if id_with_reply_for.get(id) is None]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Советую форматировать многострочные лист компрехеншены от так:

id_message_for_reply = [
    id for id in id_with_reply_for
    if id_with_reply_for.get(id) is None
]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А ещё тут условие можно упростить до if id in id_with_reply_for

А ещё id – это встроенная в питон функция, так лучше не называть переменные.

@Melevir
Copy link
Contributor

Melevir commented Jun 21, 2023

При создании пул-реквеста есть выбиралка, в какой репозиторий делать пул-реквест. Ты сделал пул-реквест в основной репо learnpythonru, так лучше не делать. Делай пр в свой репо.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants